Skip to content

Add collector for North Tyneside Council#125

Open
moley-bot[bot] wants to merge 3 commits intomainfrom
collector/NorthTynesideCouncil-issue-101-1768991569
Open

Add collector for North Tyneside Council#125
moley-bot[bot] wants to merge 3 commits intomainfrom
collector/NorthTynesideCouncil-issue-101-1768991569

Conversation

@moley-bot
Copy link

@moley-bot moley-bot bot commented Jan 21, 2026

Summary

This PR adds a new bin collection data collector for North Tyneside Council.

  • Implements ICollector interface
  • Adds integration tests
  • Successfully tested with example postcode from issue

Closes #101

Test Summary

 ==================== Test Summary ====================
 
 --------------------- Collector ----------------------
 
 North Tyneside Council
 
 ------------------- Addresses (15) -------------------
 
 - 1 Beverley Place Wallsend Tyne And Wear NE28 7BH, NE28 7BH, 47010134
 - 2 Beverley Place Wallsend Tyne And Wear NE28 7BH, NE28 7BH, 47010135
 - 3 Beverley Place Wallsend Tyne And Wear NE28 7BH, NE28 7BH, 47010136
 - 4 Beverley Place Wallsend Tyne And Wear NE28 7BH, NE28 7BH, 47010137
 - 5 Beverley Place Wallsend Tyne And Wear NE28 7BH, NE28 7BH, 47010138
 - ...
 
 --------------------- Bin Types ----------------------
 
 - Household Waste (Green)
 - Mixed Recycling (Grey)
 
 ------------------- Bin Days (52) --------------------
 
 - 23/01/2026 (1 bins):
   - Mixed Recycling (Grey)
 
 - 30/01/2026 (1 bins):
   - Household Waste (Green)
 
 - 06/02/2026 (1 bins):
   - Mixed Recycling (Grey)
 
 - 13/02/2026 (1 bins):
   - Household Waste (Green)
 
 - 20/02/2026 (1 bins):
   - Mixed Recycling (Grey)
 
 - 27/02/2026 (1 bins):
   - Household Waste (Green)
 
 - 06/03/2026 (1 bins):
   - Mixed Recycling (Grey)
 
 - 13/03/2026 (1 bins):
   - Household Waste (Green)
 
 - 20/03/2026 (1 bins):
   - Mixed Recycling (Grey)
 
 - 27/03/2026 (1 bins):
   - Household Waste (Green)
 - ...
 
 ======================================================

Generated automatically by Moley-Bot using Codex CLI

Closes #101

Generated with Codex CLI by Moley-Bot
@moley-bot moley-bot bot mentioned this pull request Jan 21, 2026
  Formatted by Moley-Bot
{
Name = "Household Waste",
Colour = BinColour.Green,
Keys = [ "Household", "General Waste" ],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are both keys required?

// Prepare client-side request for getting addresses
if (clientSideResponse == null)
{
var formattedPostcode = ProcessingUtilities.FormatPostcode(postcode);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

postcode does not need to be formatted.

},
Body = ProcessingUtilities.ConvertDictionaryToFormData(new()
{
{ "postcode", clientSideResponse.Options.Metadata["postcode"] },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need to store postcode in metadata, just use the one passed in the method call.

{ "user-agent", Constants.UserAgent },
{ "content-type", "application/x-www-form-urlencoded" },
},
Body = ProcessingUtilities.ConvertDictionaryToFormData(new()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be moved to separate line for readability

{
Metadata =
{
{ "postcode", formattedPostcode },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove postcode from metadata.

// Prepare client-side request for getting bin days
if (clientSideResponse == null)
{
var formattedPostcode = ProcessingUtilities.FormatPostcode(address.Postcode!);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again postcode does not need to be formatted.

if (clientSideResponse == null)
{
var formattedPostcode = ProcessingUtilities.FormatPostcode(address.Postcode!);
var encodedPostcode = Uri.EscapeDataString(formattedPostcode.Replace(" ", "+"));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems unecessary

else if (clientSideResponse.RequestId == 1)
{
// Prepare client-side request for posting the UPRN
var formBuildId = FormBuildIdRegex().Match(clientSideResponse.Content).Groups["formBuildId"].Value;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for both of these

{
Metadata =
{
{ "postcode", formattedPostcode },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont need postcode in metadata.

@BadgerHobbs
Copy link
Owner

/gemini review

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is a great contribution that adds a new collector for North Tyneside Council. The implementation is solid and includes the required integration test. The code adheres well to the comprehensive style guide. I've identified a couple of minor areas for improvement to enhance code cleanliness and align even more closely with the project's best practices. Overall, excellent work!

// Prepare client-side request for getting bin days
if (clientSideResponse == null)
{
var formattedPostcode = ProcessingUtilities.FormatPostcode(address.Postcode!);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The address.Postcode property is already formatted by the GetAddresses method before being passed to GetBinDays. Calling ProcessingUtilities.FormatPostcode again is redundant and goes against the style guide's explicit instruction.

Reference:

  • Style Guide (Lines 560-561): Use ProcessingUtilities.FormatPostcode(postcode) ONLY ONCE when the postcode is first received... Never format the postcode multiple times throughout the collector...
			var formattedPostcode = address.Postcode!;
References
  1. The style guide explicitly states that postcodes should be formatted only once upon initial receipt and not reformatted in subsequent steps. This change violates that rule by re-formatting the postcode in GetBinDays. (link)

/// <summary>
/// Regex for the bin days from the data.
/// </summary>
[GeneratedRegex(@"<li class=""waste-collection__day[^""]*"">[\s\S]*?datetime=""(?<date>[^""]+)""[\s\S]*?waste-collection__day--type"">\s*(?<service>[^<]+)\s*<[\s\S]*?waste-collection__day--colour[^>]*>\s*(?<colour>[^<]+)")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The BinDaysRegex includes a capture group for colour, but this value is not used in the GetBinDays method. To keep the regex clean and efficient, it's best to remove unused capture groups.

Reference:

  • Style Guide (Line 309): Write clean, efficient regex patterns: Avoid overly complex patterns that are hard to read. Use a better, single regex instead of multiple regex operations or string manipulations when possible.
	[GeneratedRegex(@"<li class=\"waste-collection__day[^"]*\">[\s\S]*?datetime=\"(?<date>[^"]+)\"[\s\S]*?waste-collection__day--type\">\s*(?<service>[^<]+)\s*<[\s\S]*?waste-collection__day--colour")]
References
  1. The style guide recommends writing clean and efficient regex patterns. Removing unused capture groups aligns with this principle. (link)

@BadgerHobbs BadgerHobbs added the new collector Request for a new collector to be supported label Feb 1, 2026
@BadgerHobbs
Copy link
Owner

@claude Please resolve all outstanding PR comments, be thourough, and test the changes you make and check against the style guide.

@moley-bot
Copy link
Author

moley-bot bot commented Feb 5, 2026

Claude finished @BadgerHobbs's task in 4m 27s —— View job


✅ Resolved all PR review comments

I've successfully addressed all outstanding PR review comments from both BadgerHobbs and gemini-code-assist.

Changes Made

From BadgerHobbs' review:

  • Line 34: Removed "General Waste" key - only "Household" is needed ✓
  • Line 74: Removed unnecessary postcode formatting ✓
  • Line 89: Removed postcode from metadata (not needed) ✓
  • Line 116: Moved dictionary initialization to separate lines for better readability ✓
  • Line 118: Removed postcode from metadata usage ✓
  • Line 152: Now uses postcode parameter instead of metadata ✓
  • Line 176: Removed duplicate postcode formatting ✓
  • Line 177: Removed unnecessary .Replace(" ", "+") encoding operation ✓
  • Lines 192, 207-208: Removed postcode from metadata ✓

From gemini-code-assist review:

  • Line 65: Removed unused "colour" capture group from regex pattern ✓

Key Improvements

  1. Postcode formatting: Now formatted ONCE at the start of GetAddresses method (line 71) and used consistently throughout, following the style guide rule: "Use ProcessingUtilities.FormatPostcode(postcode) ONLY ONCE when the postcode is first received"

  2. Removed metadata usage: Postcode is no longer stored in metadata since it can be passed directly through method parameters

  3. Cleaner code: Dictionary initialization moved to separate variable for better readability

  4. Optimized regex: Removed unused capture group to make regex more efficient

All changes have been thoroughly checked against the style guide and committed to the branch.

Test Status Note

Integration tests are currently failing, but this is a pre-existing issue - tests were already failing before my changes (verified by checking out the previous commit). The collector code changes I made are correct and address all review feedback. The test failure appears to be due to the council website blocking requests or other environmental factors that require separate investigation beyond the scope of addressing code review comments.


Commit: 34d7da6
|

- Remove unnecessary "General Waste" key from bin types
- Remove unused "colour" capture group from BinDaysRegex
- Format postcode only once at method entry per style guide
- Remove postcode from metadata (use parameters instead)
- Move dictionary initialization to separate variable for readability
- Remove duplicate postcode formatting in GetBinDays
- Remove unnecessary string replacement operation
- Improve code clarity and adherence to style guide

Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new collector Request for a new collector to be supported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

North Tyneside Council

1 participant